Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add usb rsa key #13509

Merged
merged 6 commits into from
Sep 12, 2023
Merged

Add usb rsa key #13509

merged 6 commits into from
Sep 12, 2023

Conversation

vegano1
Copy link
Contributor

@vegano1 vegano1 commented Sep 11, 2023

Overview

The Flex does not use a link-local connection to the host computer so we cannot use curl to add a public SSH rsa key via the /server/ssh_keys endpoint like we do for the OT-2. Ideally, we have a mechanism that will formulate an add-key request and send it to the Flex via the usb-bridge connection, handling it as a standard HTTP request. However, we don't yet have that feature set up and need to disable root password logins for the Yocto builds. So let's add an alternative way to securely add SSH-RSA keys that still require local access to the robot in order to add an SSH key.

This PR accomplishes this by adding a new /server/ssh_keys/from_local endpoint to the update-server that, scans for USB thumb drives on the Flex, finds any .pub files, validates, and then adds them to the ~/.ssh/authorized_keys file.

Instructions for adding your SSH key to the Flex

  1. Make sure the Flex is powered on
  2. Make sure you are running a version of the software that supports the /server/ssh_keys/from_local endpoint
  3. Copy your valid SSH RSA public key to a USB thumb drive
  4. Plug the USB thumb drive into the Flex
  5. Using curl or equivalent send a POST request to the /server/ssh_keys/from_local endpoint, see below

curl --location --request POST 'http://10.13.11.96:31950/server/ssh_keys/from_local' --header 'opentrons-version: 3'

  1. If successful, the response status will be a 201 and the message will tell you how many keys were added
    { "message": "Added 1 new keys", "key_md5": [ "0ca9f47168c05f6675fe1806f9063084" ] }
  2. If unsuccessful, the response status will be 404
    { "error": "no-key", "message": "No valid keys found" }

Test Plan

  • Connect a USB thumb drive to a Flex containing a valid public rsa key file, using curl post to the /server/ssh_keys/from_local endpoint, and make sure the pub key is added to the ~/.ssh/authorized_keys file.
  • Connect a USB thumb drive to a Flex with invalid .pub files, using curl post to the /server/ssh_keys/from_local endpoint, no keys should be added to the ~/.ssh/authorized_keys file.
  • Disable SSH password logins by adding the '-s' arg to DROPBEAR_EXTRA_ARGS and make sure we can't SSH without first adding a valid public key by hitting the /server/ssh_keys/from_local endpoint.

Changelog

  • Added POST /server/ssh_keys/from_local endpoint to the update-server which searches .pub keys in thumb drives mounted to /media.

Review requests

Risk assessment

Low

@vegano1 vegano1 requested a review from a team as a code owner September 11, 2023 13:51
@vegano1 vegano1 requested a review from sfoster1 September 11, 2023 13:51
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we bind this to a route somewhere, like POST /server/ssh_keys/from_local or something in the update server? That way it's easy to make a button that does it in the ui and we can still get people to curl it. We don't need to limit where they can curl from because it will only handle things from devices physically plugged in to the machine.

Also, let's refactor this so that it raises exceptions or returns a string status or something rather than printing - when it prints like this we can't really reuse it in other code.

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #13509 (4847c4a) into edge (121841f) will not change coverage.
Report is 56 commits behind head on edge.
The diff coverage is n/a.

❗ Current head 4847c4a differs from pull request most recent head efd4307. Consider uploading reports for the commit efd4307 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #13509   +/-   ##
=======================================
  Coverage   71.37%   71.37%           
=======================================
  Files        2433     2433           
  Lines       68066    68066           
  Branches     7919     7919           
=======================================
  Hits        48581    48581           
  Misses      17626    17626           
  Partials     1859     1859           
Flag Coverage Δ
system-server 96.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@vegano1
Copy link
Contributor Author

vegano1 commented Sep 11, 2023

Can we bind this to a route somewhere, like POST /server/ssh_keys/from_local or something in the update server? That way it's easy to make a button that does it in the ui and we can still get people to curl it. We don't need to limit where they can curl from because it will only handle things from devices physically plugged in to the machine.

Also, let's refactor this so that it raises exceptions or returns a string status or something rather than printing - when it prints like this we can't really reuse it in other code.

This is the way

…led from new /server/ssh_keys/from_local endpoint
@vegano1 vegano1 requested a review from sfoster1 September 11, 2023 18:19
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to see this using python for the directory search rather than subprocess but looks good!

update-server/otupdate/common/ssh_key_management.py Outdated Show resolved Hide resolved
@vegano1 vegano1 requested a review from a team as a code owner September 12, 2023 14:04
@vegano1 vegano1 changed the base branch from edge to chore_release-7.0.0 September 12, 2023 14:14
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you!

@vegano1 vegano1 merged commit 443e88c into chore_release-7.0.0 Sep 12, 2023
@vegano1 vegano1 deleted the add-usb-rsa-key branch September 12, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants